Skip to content

chore(layers): add Idempotency, Batch, and Parameters to layer #1712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

am29d
Copy link
Contributor

@am29d am29d commented Sep 25, 2023

Description of your changes

This PR adds idempotency utilities to the public layer. Thanks to the outstanding work in #1708 and #1710 this PR became lightweight.

I decided to add sdk clients to the tests, since it was a major concern as thoroughly discussed in #1708 .

Related issues, RFCs

Issue number: closes #1703

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my change is effective and works
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@am29d am29d requested a review from a team September 25, 2023 19:40
@boring-cyborg boring-cyborg bot added layers Items related to the Lambda Layers pipeline tests PRs that add or change tests labels Sep 25, 2023
@pull-request-size pull-request-size bot added the size/M PR between 30-99 LOC label Sep 25, 2023
@am29d am29d requested review from dreamorosi and removed request for a team September 25, 2023 19:40
@am29d am29d self-assigned this Sep 25, 2023
@am29d am29d changed the title chore(layer): add parameters, batch, idempotency to layer chore(layers): add parameters, batch, idempotency to layer Sep 25, 2023
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration tests for Node.js 14 & 16 are failing on the layer module (link).

Looking at the CloudWatch logs (see this LogGroup /aws/lambda/Layers-E2E-node14-11177-functionStack-testFn) it appears the SDK is not being bundled on these versions.

Check here, I'm not sure which runtime the Lambda deployed for these tests is actually using.

@am29d
Copy link
Contributor Author

am29d commented Sep 26, 2023

I have added test groups for node14 and node16 and we have a similar problem we had before. AWS SDK clients are bundled by esbuild, the layer has all utilities, but the imports of AWS SDK can not be found.

I am digging into esbuild and CDK bundling, and will also check SAM examples.

@am29d am29d added the do-not-merge This item should not be merged label Sep 26, 2023
@am29d am29d changed the title chore(layers): add parameters, batch, idempotency to layer chore(layers): add idempotency to layer Sep 26, 2023
@dreamorosi dreamorosi self-assigned this Sep 26, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dreamorosi dreamorosi self-requested a review September 26, 2023 13:11
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreamorosi dreamorosi changed the title chore(layers): add idempotency to layer chore(layers): add Idempotency, Batch, and Parameters to layer Sep 26, 2023
@dreamorosi
Copy link
Contributor

For the record and future readers, at this point the Lambda Layer includes all Powertools utilities and the AWS SDK clients for the services used by them (SSM, Secrets Manager, DynamoDB, and AppConfig).

The total size is 10MB unzipped, we were able to shave off 8MB (-45%) by removing types, and ESM distributions of the SDKs that are shipped, and other files not needed at runtime that are shipped with some of the dependencies.

There's a potential for additional optimization that would allow to decrease the Layer size by 35-50% (based on initial tests), however we have decided to not pursue it at this time mainly because of other competing priorities.

The optimization would involve creating a barrel file (i.e. sdk/index.js) that imports and re-exports only the clients and methods of the SDK used in Powertools, i.e. :

import {
  AttributeValue,
  DeleteItemCommand,
  DynamoDBClient,
  DynamoDBClientConfig,
  DynamoDBServiceException,
  GetItemCommand,
  PutItemCommand,
  UpdateItemCommand,
} from '@aws-sdk/client-dynamodb';
import { marshall, unmarshall } from '@aws-sdk/util-dynamodb';

export {
  AttributeValue,
  DeleteItemCommand,
  DynamoDBClient,
  DynamoDBClientConfig,
  DynamoDBServiceException,
  GetItemCommand,
  marshall,
  PutItemCommand,
  unmarshall,
  UpdateItemCommand,
};

the file would then be run through esbuild like:

npx esbuild sdk/index.js --bundle --outfile=tmp/nodejs/node_modules/@aws-lambda-powertools/commons/lib/sdk/index.js --platform=node --target=node14

this would generate an optimized (& potentially minified) version of the SDK that bundles only the code needed. We would then have to programmatically change the imports within the Powertools code to require this file rather than the SDK, i.e.

const client_dynamodb_1 = require("@aws-sdk/client-dynamodb");
const util_dynamodb_1 = require("@aws-sdk/util-dynamodb");

would become

const client_dynamodb_1 = require("../../../commons/lib/sdk");
const util_dynamodb_1 = require("../../../commons/lib/sdk");

We were able to test this and confirm that: 1/ a Layer created with this method works with all Node.js runtimes, 2/ the total size is significantly lower than the current one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layers Items related to the Lambda Layers pipeline size/M PR between 30-99 LOC tests PRs that add or change tests
Projects
None yet
2 participants